Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow the CLI to override options set in the config file #117

Merged
merged 2 commits into from
May 8, 2017
Merged

Allow the CLI to override options set in the config file #117

merged 2 commits into from
May 8, 2017

Conversation

DavidS
Copy link
Contributor

@DavidS DavidS commented May 5, 2017

This follows the generally expected precedence of the CLI options
overriding values from the config file. In particular, I found this when
trying to create a travis job for modulesync_config
(see https://travis-ci.org/voxpupuli/modulesync_config/jobs/229043830).

@DavidS DavidS mentioned this pull request May 5, 2017
Merged
@domcleal
Copy link
Contributor

domcleal commented May 5, 2017

The remote_branch command line option should default to the config file value too, according to README.md. VP's modulesync_config also has message set, but this isn't documented - it might be reasonable to default the message option to the config file value if set and document it.

Lastly, pre_commit_script is a documented config file option but it isn't exposed through the CLI arguments. I think as a result of removing the config.merge!(config_file), this will no longer be populated from the config file. It'll either need to be loaded into the options hash from defaults or exposed as a CLI option, defaulting to the config file value.

It might be worth checking through the source for any more undocumented options - sorry!

DavidS added 2 commits May 5, 2017 17:23
@DavidS
Minor README fixups
a8eb8d9
@DavidS
Allow the CLI to override options set in the config file
c25a2f0 
This follows the generally expected precedence of the CLI options
overriding values from the config file. In particular, I found this when
trying to create a travis job for modulesync_config
(see https://travis-ci.org/voxpupuli/modulesync_config/jobs/229043830).

Adds the "missing" pre_commit_script CLI option for the documented
override of the same name.

A potentially breaking change is that now only documented overrides are
allowed.
@DavidS
Copy link
Contributor Author

DavidS commented May 5, 2017

@domcleal fixed and travissed!

@domcleal domcleal merged commit 0a83bc7 into voxpupuli:master May 8, 2017
@domcleal
Copy link
Contributor

domcleal commented May 8, 2017

Thanks @DavidS!

@DavidS DavidS deleted the fix-cli-precedence branch May 8, 2017 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DavidS @domcleal